feat(theming): add theme mode system (Classic, Minimal, E-Ink) - #2423
feat(theming): add theme mode system (Classic, Minimal, E-Ink)#2423MarvNC wants to merge 22 commits into
Conversation
…cy and pitch tags
…nk tags Remove dynamic theme stylesheet loading from display.js and popup.js. Theme CSS files are now loaded statically via <link> tags in popup.html, search.html, and popup-preview.html. The active theme is controlled by the data-theme-mode attribute on <html>, set by theme-controller.js. This simplifies adding new themes to three steps: 1. Create CSS file with :root[data-theme-mode='id'] selectors 2. Add <link> tag to HTML pages 3. Register in theme-registry.js No JavaScript changes required per theme.
Add comprehensive documentation for creating Yomitan themes, covering the additive CSS architecture, 5-step creation process, CSS patterns, best practices, and testing. Link from README under Developer Documentation → Advanced Features.
…text CSS loading Fix review findings: restore theme-registry.js with css property, add outer chrome injection to popup.js, generate dropdowns dynamically from registry in settings and welcome pages. Theme CSS files now contain both inner (:root) and outer (iframe) selectors — loaded statically via <link> for inner content, injected dynamically for outer chrome. - Add css property back to theme-registry.js - Add _injectThemeStylesheet() to popup.js for outer DOM injection - Generate theme dropdowns from registry in settings-main.js + welcome-main.js - Move iframe rules back into theme-minimal.css and theme-eink.css - Fix .menu → .popup-menu in e-ink border-radius block - Add .progress-bar-indeterminant to e-ink animation reset - Update docs/theming.md with correct dual-context architecture
…r injection Fix critical issues found in code review: - Populate theme dropdown options BEFORE GenericSettingController init, so saved values are applied correctly (was populating after, causing empty select to default to first option) - Replace loadStyle with manual DOM injection in popup.js outer chrome. loadStyle's WebExtension API tracking throws on repeated injection with the same ID, crashing theme switching in non-shadow mode. - Update theme-minimal.css tag variable names to match display.css refactor: --tag-muted-background-color and --tag-accent-background-color (was overriding dead --tag-default-background-color etc.) - Remove unnecessary async from Display._setTheme (no await inside) - Update docs/theming.md schema default to minimal (matching actual schema)
…ity, registry-driven shadow
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fa6e60622
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Inject outer theme CSS after shadow root exists (P1) _injectStyles() now calls _injectThemeStylesheet() after _setUpContainer creates the shadow root, ensuring Minimal/E-Ink outer chrome styles apply. - Make dark outer-theme shadow variable-driven (P2) Changed hard-coded white glow to var(--popup-box-shadow, ...) so themes can override it while preserving the Classic default fallback.
|
@codex review all commits |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fa6e60622
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR introduces a theme mode system layered on top of the existing theme settings, letting users choose between three CSS-driven presentations of popup and search content (Classic, Minimal, E-Ink). The feature is implemented as additive CSS files keyed on a new data-theme-mode attribute set by ThemeController, with a registry that drives both the settings/welcome dropdowns and dynamic outer-chrome stylesheet injection in popup.js. A new general.popupThemeMode option is added to the schema, types, and migrated via options v77 (existing users → classic, new installs → minimal).
Changes:
- Add
popupThemeModeoption (schema + types + v77 migration) and propagate it throughThemeController,Display._setTheme, andPopup._setOptionsContext(including dynamic outer theme stylesheet injection). - Add theme registry (
ext/js/data/theme-registry.js), two new CSS theme files (theme-minimal.css,theme-eink.css), corresponding<link>tags inpopup.html/search.html/popup-preview.html, and CSS variable additions todisplay.css/popup-outer.css. - Add Mode dropdown (registry-populated) in Settings and Welcome pages, with logic that forces Shadow →
noneand disables the Shadow select when E-Ink is selected; adddocs/theming.mdand README link.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| types/ext/settings.d.ts | Adds PopupThemeMode union and popupThemeMode field on GeneralOptions. |
| ext/data/schemas/options-schema.json | Adds popupThemeMode enum/required field (default minimal). |
| ext/js/data/options-util.js | Adds v77 migration setting existing profiles to popupThemeMode='classic'. |
| test/options-util.test.js | Updates test fixtures to expect v77 and popupThemeMode: 'classic'. |
| ext/js/data/theme-registry.js | New registry of themes plus getThemeById / populateThemeModeSelect. |
| ext/js/app/theme-controller.js | Adds themeMode getter/setter and writes data-theme-mode to target element. |
| ext/js/app/popup.js | Reads popupThemeMode and dynamically injects outer theme CSS into popup chrome. |
| ext/js/display/display.js | Passes popupThemeMode to ThemeController for inner content theming. |
| ext/js/pages/settings/settings-main.js | Populates theme-mode dropdown and manages E-Ink → Shadow disable logic. |
| ext/js/pages/welcome-main.js | Populates theme-mode dropdown on the welcome page. |
| ext/settings.html / ext/welcome.html | Adds Mode dropdown UI and id hooks. |
| ext/popup.html / ext/search.html / ext/popup-preview.html | Statically links the new theme CSS files. |
| ext/css/theme-minimal.css / theme-eink.css | New theme override stylesheets (inner + outer selectors). |
| ext/css/display.css | Adds --tag-muted-background-color / --tag-accent-background-color variables and uses them. |
| ext/css/popup-outer.css | Switches hard-coded popup chrome values to CSS variables. |
| docs/theming.md / README.md | New theming guide and README link. |
| package.json | Adds --ignore-pattern '**/*.jsonc' to test:js. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Defer theme stylesheet injection until popup container is ready (only inject in _setOptionsContext when _injectPromiseComplete) - Enforce E-Ink outerTheme='none' at runtime to prevent shadows - Remove redundant manual stylesheet removal; let loadStyle manage cache - Remove unnecessary void operator before synchronous _setTheme - Prevent settings UI from overwriting saved popupOuterTheme on load (add dispatchChange parameter to updateThemeModeUI) Refs: yomidevs#2423
- Defer theme stylesheet injection until popup container is ready (only inject in _setOptionsContext when _injectPromiseComplete) - Enforce E-Ink outerTheme='none' at runtime to prevent shadows - Remove redundant manual stylesheet removal; let loadStyle manage cache - Remove unnecessary void operator before synchronous _setTheme - Prevent settings UI from overwriting saved popupOuterTheme on load (add dispatchChange parameter to updateThemeModeUI) Refs: yomidevs#2423
Summary
This PR introduces a theme mode system for Yomitan, allowing users to choose between three visual styles for popups and the search page:
Screenshots
Architecture
Additive CSS with
data-theme-modeThemes are implemented as additive CSS overrides on top of the base Classic theme:
display.css,material.css, etc. (Classic is the default)theme-minimal.css,theme-eink.css):root[data-theme-mode='...']selectors — only the active theme's rules match<link>tags; thedata-theme-modeattribute on<html>controls activationDual-context loading
Each theme CSS file contains selectors for both rendering contexts:
:root[data-theme-mode='...']) — styles content inside the popup iframe, loaded staticallyiframe.yomitan-popup[data-theme-mode='...']) — styles the iframe element itself (borders, shadows, radius), injected dynamically bypopup.jsinto the parent page's shadow DOMThis means one file per theme, loaded in both contexts. Selectors that don't match are harmless no-ops.
Registry-driven
ext/js/data/theme-registry.jsis the single source of truth:The registry drives:
<option>generation in Settings and Welcome pagespopup.jsAdding a new theme requires only 4 steps:
theme-new.css(inner + outer selectors)<link>topopup.html,search.html,popup-preview.htmltheme-registry.jsTheme Details
Minimal
E-Ink
Settings
New "Mode" selector in Appearance settings (and Welcome page):
Migration
Existing users are automatically migrated to Classic via options v77 to preserve their current appearance. New installations default to Minimal.
Documentation
Added
docs/theming.md— a comprehensive guide for creating custom themes.Testing
npm run test:fast— 4,136 tests passednpm run test:css— stylelint cleanFiles Changed
Core:
ext/js/data/theme-registry.js— Theme definitionsext/js/app/theme-controller.js—data-theme-modeattribute managementext/js/app/popup.js— Outer chrome injectionext/js/display/display.js— Inner content theme applicationCSS:
ext/css/theme-minimal.css— Minimal theme overridesext/css/theme-eink.css— E-ink theme overridesext/css/display.css— CSS variable additions for themingSettings:
ext/settings.html— Theme mode selectorext/welcome.html— Welcome page theme selectorext/js/pages/settings/settings-main.js— Dropdown generation, e-ink handlingext/js/pages/welcome-main.js— Welcome page dropdown generationSchema/Types:
ext/data/schemas/options-schema.json—popupThemeModefieldtypes/ext/settings.d.ts—PopupThemeModetypeext/js/data/options-util.js— v77 migration (default to classic)Docs:
docs/theming.md— Theme creation guideREADME.md— Link to theming docs